Handle CR without NL printed in EditorConsole #9954
Conversation
|
LOVE. THIS. PR |
|
Nice! Definitely I want to see this feature implemented!
I'd like to see also a test when multiple lines are sent together like:
You can't run a single test from |
Good point. I might have already tested this, but let's do this regardless.
Ah, I don't have eclipse set up anymore, so that's not an easy way out here :-) |
matthijskooijman
force-pushed
the
console-single-cr
branch
from
4b24c45 to
05aa839
Compare
|
Ok, just pushed an update. I rebased on top of master and added a fixup commit with some improvements and a testcase. Took me a bit longer than expected after I started on this two days ago, but I produced a few more PRs with test-suite fixes and repo cleanup along the way, so that's nice :-p As far as I'm concerned, I think this is now ready to merge (after squashing the fixup commit, of course). |
matthijskooijman
force-pushed
the
console-single-cr
branch
from
05aa839 to
540d028
Compare
Not trimming makes this method usable for unittesting. Since this method is not currently used anywhere, it should not affect any behaviour.
This allows testing this class without going to a full initialization.
…onsole Previously, the redirection would be triggered in the EditorConsole constructor. However, this was problematic for unittests, which do not need this redirection. Since the redirection really is not useful intul there is a current EditorConsole anyway, it can just be delayed a bit until setCurrentEditorConsole is called.
Previously, any CR without NL was treated just like a NL. For tools that used single CRs to update a progress bar (such as dfu-util), this would end up printing the subsequent versions of the progress bar below each other, instead of updating a single line as intended. Additionally, since ConsoleOutputStream only scrolled the view on \n, these updates would end up outside of the main view, making the upload progress quite unclear. This commit makes EditorConsole support lone CRs by resetting the insert position to the start of the current line, so subsequent writes overwrite existing content. If subsequent lines are shorter than an earlier line, only part of the earlier line will be overwritten (this mimics what terminal emulators do).
matthijskooijman
force-pushed
the
console-single-cr
branch
from
540d028 to
1a358a3
Compare
|
Just amended some more style fixes into the fixup commit and prepended some more commits to |
|
✅ Build completed. Please test this code using one of the following: ⬇️ https://downloads.arduino.cc/javaide/pull_requests/arduino-PR-9954-BUILD-962-linux32.tar.xz ℹ️ The |
|
Rebased and merged 🎉 |
per1234
added
Component: IDE
When uploading a sketch using dfu-util (e.g. used by various STM32 cores), progress bar lines would be printed. However, each update would be printed on a new line, rather than overwriting the same line as intended. Even more, the console would not scroll when subsequent lines were added, which made it quite annoying to see the upload progress.
It turns out dfu-util uses lone
\rto overwrite the previous line. However,EditorConsolewould simply replace those with\ninstead. But becauseConsoleOutputStreamonly scrolled on\n, these extra lines would fall out of view on the bottom of the console.I originally tried updating the scroll behaviour to include these lines, but then thought it would be even nicer to just properly handle
\rcharacters instead. That took a bit more time than I had anticipated, but the result is fairly clean code, I think :-)With this PR applied, the dfu-util now looks like this:
Without this PR, it was like this (note that most of the action happens outside of view below, until the upload is complete):
As an additional test for partial line overwriting, I added this bit of shell script to a script that was used during upload:
Which works as expected:
Again, this would be a good candidate for some testcases. @cmaglie, do you have some notes you could share about how to run the testsuite in a meaningful way (especially just run a single testcase)?